Skip to content

fix(agent): clear ordered thinking replay state on signature recovery - #70107

Open
Eklps wants to merge 1 commit into
NousResearch:mainfrom
Eklps:fix/sig-recovery-ordered-blocks
Open

Eklps wants to merge 1 commit into
NousResearch:mainfrom
Eklps:fix/sig-recovery-ordered-blocks

Conversation

@Eklps

@Eklps Eklps commented Jul 23, 2026

Copy link
Copy Markdown

Summary

Follow-up to #66694.

On Invalid signature in thinking block recovery, Hermes previously stripped only reasoning_details from API-time message copies but retained anthropic_content_blocks — a separate ordered-block channel that can replay the same invalid signed thinking block, causing the retry to 400 again.

This clears both fields from api_messages while leaving the canonical messages store untouched (session history is preserved).

Root cause

When a provider returns HTTP 400 Invalid signature in thinking block, the recovery path in conversation_loop.py strips reasoning_details from shallow-copied api_messages and retries. However, build_assistant_message also lifts anthropic_content_blocks (the verbatim, order-preserving Anthropic content block list) onto the stored message. The replay fast-path in convert_messages_to_anthropic sources from this channel when present — so the retry still replayed the same invalid signed thinking block and 400'd again.

Fix

Strip anthropic_content_blocks from API-time copies alongside reasoning_details during signature recovery. The canonical messages list (and therefore state.db) is unchanged.

Verification

  • python -m py_compile — passed
  • git diff --check — passed
  • tests/run_agent/test_thinking_sig_recovery_persistence.py — 4 passed (updated to cover both reasoning_details and anthropic_content_blocks)

Scope

Two files:

  • agent/conversation_loop.py — add anthropic_content_blocks to the recovery strip
  • tests/run_agent/test_thinking_sig_recovery_persistence.py — update tests to cover both fields

No changes to agent/anthropic_adapter.py, agent/transports/, or any provider plugin.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint provider/anthropic Anthropic native Messages API labels Jul 23, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for tracing the ordered-block replay channel. The premise holds on current main: agent/conversation_loop.py:3942-3945 strips only reasoning_details, while agent/anthropic_adapter.py:2044-2102 preferentially replays non-empty anthropic_content_blocks; agent/chat_completion_helpers.py:1541-1551 stores that channel on assistant messages.

Problems

  • tests/run_agent/test_thinking_sig_recovery_persistence.py mirrors the stripping loop rather than invoking agent/conversation_loop.py:3936-3957 and the Anthropic conversion path. It therefore cannot catch a wiring regression where the production retry still replays anthropic_content_blocks.

Suggested changes

  • Add a real-path recovery test that verifies the retry wire payload drops both fields, canonical messages retain both fields, and convert_messages_to_anthropic() cannot select the ordered replay path for that retry.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 30, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Two open PRs address the same invalid signed-thinking replay failure during signature recovery. #24407 strips thinking blocks from canonical messages, while #70107 clears both replay channels from API-time copies and preserves canonical history.

Related pull requests

  • fix(agent): strip thinking blocks from content arrays on signature recovery #24407 duplicate — (+145/-7) — verify verdict: n/a: The diff removes thinking and redacted_thinking blocks from assistant content arrays and clears reasoning_details, directly targeting the content-array failure, but mutates canonical messages and tests a reproduced loop rather than the production retry/conversion path. The automated keep-open verdict records medium salvageability and identifies moving the filtering to api_messages plus a real-path test as necessary.
  • fix(agent): clear ordered thinking replay state on signature recovery #70107 related — (+48/-10) — verify verdict: n/a: The diff removes reasoning_details and anthropic_content_blocks from shallow-copied api_messages, addressing the ordered replay channel while leaving canonical messages unchanged; its tests still mirror the stripping loop instead of exercising production retry and Anthropic conversion. The automated keep-open verdict records high salvageability and requests a real-path test covering the clean wire payload, preserved canonical state, and disabled ordered replay.

Duplicates

#24407 and #70107 substantially overlap: both target repeated HTTP 400 failures caused by replayed signed thinking state during recovery. #24407 is the broader content-array variant; #70107 targets the current API-time ordered-replay architecture.

Suggested consolidation

Keep open #70107 with a salvage path: retain its API-time clearing of both replay channels and add the requested real-path retry/conversion test. Close #24407 as duplicate of #70107 after preserving its content-array diagnosis; this differs from #24407's automated keep-open verdict, but the recorded diff shows canonical-history mutation and the same fix must be relocated to the API-time path.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    subgraph Dup24407 ["PRs duplicating each other"]
        P24407["PR #24407 (open)"]
        P70107["PR #70107 (open)"]
    end
    class P24407 open
    class P70107 open
    class P70107 target
    click P24407 "https://github.com/NousResearch/hermes-agent/pull/24407"
    click P70107 "https://github.com/NousResearch/hermes-agent/pull/70107"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 15 kB of PR diffs, 5 kB of issue/PR text, 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

On Invalid signature in thinking block recovery, Hermes previously stripped
only reasoning_details from API-time message copies but retained
anthropic_content_blocks — a separate ordered-block channel that can replay
the same invalid signed thinking block, causing the retry to 400 again.

This clears both fields from api_messages while leaving the canonical messages
store untouched (session history is preserved).

Adds a real-path test that exercises convert_messages_to_anthropic() to verify
the ordered-block replay fast path does not fire after recovery stripping,
while canonical messages retain both fields.

Addresses review feedback on NousResearch#70107 from @teknium1 and triage bot.
@Eklps
Eklps force-pushed the fix/sig-recovery-ordered-blocks branch from fca586e to e78c9f6 Compare August 4, 2026 04:52
@Eklps

Eklps commented Aug 4, 2026

Copy link
Copy Markdown
Author

Updated with real-path test coverage as requested by @teknium1 and triage review.

The new test_recovery_strips_ordered_blocks_from_retry_wire_payload exercises the production path:

  1. Before recovery: builds assistant message with both reasoning_details and anthropic_content_blocks (signed thinking + tool_use), then calls convert_messages_to_anthropic() — confirms the ordered-block fast path fires and replays signed thinking.
  2. After recovery: shallow-copies to api_messages, strips both fields, calls convert_messages_to_anthropic() again — confirms the ordered-block fast path does NOT fire (no thinking block in output), while tool_use survives via reconstruction from tool_calls.
  3. Canonical messages: verifies both fields are still intact on the original messages list.

Rebased onto current upstream/main (70db671). All 5 tests pass.

@alt-glitch alt-glitch removed the sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) label Aug 4, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists provider/anthropic Anthropic native Messages API sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants